Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Keep milestones when switching repo #311

Merged
merged 6 commits into from
Mar 27, 2024
Merged

Keep milestones when switching repo #311

merged 6 commits into from
Mar 27, 2024

Conversation

Arif-Khalid
Copy link
Contributor

@Arif-Khalid Arif-Khalid commented Mar 20, 2024 •
edited
Loading

Summary:

Fixes part of #205

Type of change:

  • ✨ New Feature/ Enhancement

Changes Made:

  • Changed behaviour on new repo initialization to sanitize current milestones instead of replacing them
  • The result of saving 0 eligible milestones resets to selecting all of them
  • The result of saving all milestones from previous repo selects all milestones in new repo

Proposed Commit Message

Milestones are not saved even when filters are kept.

This is inconsistent with the meaning of keeping filters.

Let's implement keeping milestones across repos.

Checklist:
  • I have tested my changes thoroughly.
  • I have created tests for any new code files created in this PR or provided a link to a issue/PR that addresses this.
  • I have added or modified code comments to improve code readability where necessary.
  • I have updated the project's documentation as necessary.

@Arif-Khalid Arif-Khalid added the category.Enhancement an enhancement to an existing feature label Mar 20, 2024
@Arif-Khalid Arif-Khalid self-assigned this Mar 20, 2024
@Arif-Khalid
Copy link
Contributor Author

FiltersService::previousMilestonesLength serves the purpose of determining whether all milestones were selected from the previous repo when saved to the new one.
This case should be handled since if the user did not deliberately select specific milestone filters, we should not deliberately select certain milestones in the new repo when filters are kept.
An example of the behaviour I am avoiding by handling this case is as follows:

  1. All milestones selected, user did not change or even notice milestone filters
    image
  2. Keep filters to new repo with no matching milestones
    image
  3. Without a milestone is exclusively selected since its title matches previous repo selection. User expects all milestones to be selected since he did not adjust milestone filters
    image

With previousMilestonesLength and the case of all milestones selected handled, step 3 correctly shows all milestones of the new repo being selected.
The correct behaviour is the current implementation of this PR.

nknguyenhc
nknguyenhc previously approved these changes Mar 23, 2024
View reviewed changes
Copy link
Contributor

@nknguyenhc nknguyenhc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I like how you also handle the edge cases in keeping the milestones.

NereusWB922
NereusWB922 previously approved these changes Mar 24, 2024
View reviewed changes
Copy link
Contributor

@NereusWB922 NereusWB922 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Arif-Khalid Arif-Khalid dismissed stale reviews from NereusWB922 and nknguyenhc via e2cd23b March 25, 2024 06:30
@nknguyenhc nknguyenhc merged commit b226977 into CATcher-org:main Mar 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nknguyenhc nknguyenhc nknguyenhc approved these changes

@NereusWB922 NereusWB922 NereusWB922 left review comments

@vigneshsankariyer1234567890 vigneshsankariyer1234567890 Awaiting requested review from vigneshsankariyer1234567890

@gycgabriel gycgabriel Awaiting requested review from gycgabriel

@luminousleek luminousleek Awaiting requested review from luminousleek

@MadLamprey MadLamprey Awaiting requested review from MadLamprey

Assignees

@Arif-Khalid Arif-Khalid

Labels
category.Enhancement an enhancement to an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Arif-Khalid @nknguyenhc @NereusWB922